Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - Try running vitest instead of jest for co2.js #235

Closed
wants to merge 2 commits into from

Conversation

mrchrisadams
Copy link
Member

@mrchrisadams mrchrisadams commented Nov 5, 2024

Triage

Type of change

Please select any of the below items that are appropriate.

This pull request:

  • Adds a new carbon estimation model to CO2.js
  • Adds new functionality to an existing model
  • Fixes a bug with an existing model implementation
  • Add other new functionality to CO2.js
  • Add new data to CO2.js
  • Improves developer experience for contributors
  • Adds contributors to CO2.js
  • Does something not specified above

Related issue/s

Please list below any issues this pull request is related to.

Docs changes required

Do any changes made in this pull request require parts of the CO2.js documentation to be updated?

  • Yes
  • No
  • I don't know

If you answered "Yes", please create an corresponding issue in our Developer Documentation repository.

Describe the changes made in this pull request

This is a test PR as an experiment in trying out Vitest, a more actively maintained testing library than Jest. It also seems to be faster to run, and was not too complicated to try migrating the exiting codebase to use it instead of jest.

Update: actually, I don't think it's that much faster to run, but the output is clearer in terms of showing where the slow tests are:

With jest

PASS src/sustainable-web-design-v3.test.js
PASS src/sustainable-web-design-v4.test.js
PASS src/co2.test.js
PASS src/hosting-api.test.js
PASS src/hosting.test.js
PASS src/1byte.test.js
PASS src/hosting-json.node.test.js
PASS src/hosting-database.node.test.js

Test Suites: 8 passed, 8 total
Tests: 113 passed, 113 total
Snapshots: 0 total
Time: 1.293 s
Ran all test suites matching /src/i.

With vitest


✓ src/co2.test.js (51 tests) 41ms
✓ src/hosting-api.test.js (6 tests) 13ms
✓ src/hosting-json.node.test.js (3 tests) 14ms
✓ src/1byte.test.js (3 tests) 4ms
✓ src/hosting-database.node.test.js (2 tests) 14ms
✓ src/hosting.test.js (9 tests) 1491ms
✓ hosting > checking a single domain with #check > use the API instead 579ms

Test
Files 8 passed (8)
Tests 113 passed (113)

Start at 18:56:56
Duration 2.10s (transform 169ms, setup 0ms, collect 433ms, tests 1.60s, environment 2ms, prepare 636ms)

Note: I think eslint relies on jest, so jest is still bundled into the dev dependencies for package.json.

As clearly as possible, describe the changes made in the pull request. You should at least detail "what changes have been made" and "what the results of these changes will be".

The main changes are

Testing Framework Migration:

  • Migrated from Jest to Vitest in package.json and updated test scripts
  • Updated test files to use Vitest imports and mocks (__mocks__/https.js)

Workflow Configuration:

  • Updated Node.js versions in .github/workflows/unittests.yml to [18.x, 20.x, 22.x] and updated actions versions

Dependency Updates:

  • Updated @tgwf/url2green to version ^0.4.1 in package.json (this was needed to run npm install successfully. It can be added separately)
  • Added vitest as a new dependency in package.json

Note: There's no need to merge this in. We probably want to break this PR up into separate changes as there are three separate kinds of changes here (bumping url2green, updating unit tests, and then adding migrating to vitest). It was just a quick experiment after I tried vitest on url2green, and wondered how much work it might be to use in co2.js.

@fershad
Copy link
Contributor

fershad commented Nov 11, 2024

Thanks @mrchrisadams I'll close this PR, and we can tackle this in three smaller PRs as you've suggested.

First priority is bumping the url2green library, and then tackle any of the testing stuff.

@fershad fershad closed this Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants